-
Notifications
You must be signed in to change notification settings - Fork 501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf(cloudflare): check for asset url #1236
Conversation
This was useful to debug KV : const list = await env.__STATIC_CONTENT.list();
console.log("GET ASSET FROM KV", list.keys.map((k) => k.name), { publicAssetBases }); Should we add e2e tests ? Especially Nuxt ones. IE deploy nuxt project with nitro + wrangler and hit the endpoints. |
This is a nice enhancenment! I think we can use existing |
I first tried to use But maybe |
Can you please explain more what routes are not being matched? |
Codecov Report
@@ Coverage Diff @@
## main #1236 +/- ##
==========================================
+ Coverage 76.58% 76.89% +0.30%
==========================================
Files 71 68 -3
Lines 7223 6829 -394
Branches 718 693 -25
==========================================
- Hits 5532 5251 -281
+ Misses 1690 1577 -113
Partials 1 1 |
Good point about also checking |
Remove KV access when an API route is called. cfm
Should return a KV hit more often. remove log chore: lint cloudflare changes
This was done in the later commits. Reminder to review again ππ½ |
Thanks for changes. As an update, i still need to revisit this issue while it is important fix and i value your efforts, i am still not convinced why we are introducing a new util like this so need to check it more in depth (i might be wrong as well!) |
Now that look more at it, this PR is more of a fix than a perf, as currently there's a bug where some matched assets aren't returned by KV, and some non existing assets are being accessed. |
With workers static assets we won't need to depend on binding anymore. |
Remove KV access when an API route is called.
π Linked issue
Fix #1001
Resolves #1235
β Type of change
π Description
π Checklist